Skip to content

Conversation

@pavelgj
Copy link
Collaborator

@pavelgj pavelgj commented Jun 18, 2024

image

@pavelgj pavelgj requested review from jba and randall77 June 18, 2024 12:43
// Generative models to provide.
// If empty, a complete list will be obtained from the service.
Models []string
Models map[string]*ai.ModelCapabilities
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the user provides both the model name and its capabilities? How do they know? What if they're wrong?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for new models that haven't been released by us yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alex, the idea here is that developers who are just starting out would leave Models blank, and the plugin would call the ListModels API and fetch all available models. It's good for developers who are just starting out, but the idea is that once they've built everything they would provide the final list of model they are using so that the framework doesn't have to do ListModels on cold start.

Reading that back, now I'm convinced that ListModels is not ideal. The bottom line is that we need ai.ModelCapabilities one way or the other. Either the user must provide them (which is bad DX and error prone) or we have to hardcode them. The approach we took on JS side is the latter -- all known models are hardcoded in the plugin with their capabilities. If a new model comes out that we're not aware of only then the developer will have to manually specify ai.ModelCapabilities (or wait until we add the model to the plugin).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like/agree with the last takeaway. Hardcoding what we know and allowing some way to use newly released models with manual settings feels like the right way to go since new models don't drop every week.

I didn't know about the ListModels API.

@pavelgj pavelgj changed the title fix: [Go] added model capabilities metadata for googleai plugin PROPOSAL: [Go] added model capabilities metadata for googleai plugin Jun 18, 2024
jba added a commit that referenced this pull request Jun 24, 2024
Every model must be associated with a set of capabilities.
DefineModel now takes an *ai.ModelCapabilities as a second argument.
It can be omitted for known models, but must be provided
for unknown ones.

Modeled on #426.
@jba
Copy link
Contributor

jba commented Jun 24, 2024

I implemented this in #456.

jba added a commit that referenced this pull request Jun 24, 2024
Every model must be associated with a set of capabilities.
DefineModel now takes an *ai.ModelCapabilities as a second argument.
It can be omitted for known models, but must be provided
for unknown ones.

Based on #426.
@pavelgj
Copy link
Collaborator Author

pavelgj commented Jun 27, 2024

this is superseded by #478

@pavelgj pavelgj closed this Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants